-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(ourlogs): Add auto-refresh to logs #94887
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This adds an auto-refresh toggle behind the 'ourlogs-live-refresh' flag. Auto-refresh can be enabled with filters so long as the sort is by time and descending. It works on 5 second polling, with a virtual time using RAF to emulate streaming. Filters are applied so data is queried in a ~30s second window behind our ingest delay. Auto-refresh will automatically disable itself under the following conditions: - You've hit a rate limit consistently over 3 polls (>1k logs/s) - You've hit an api error for any reason - You've hit the absolute timeout cap (10 minutes), you can re-enable it if you're actively using it - You change sort / filter etc.
eaeb4a4
to
cc21771
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Time Sort Check Fails for Descending Order
The checkSortIsTimeBasedDescending
function incorrectly checks if any sort is descending, rather than verifying that the time-based sort (obtained from getTimeBasedSortBy
) is specifically descending. This can lead to auto-refresh being enabled when the time-based sort is ascending, causing incorrect behavior.
static/app/views/explore/logs/utils.tsx#L198-L204
sentry/static/app/views/explore/logs/utils.tsx
Lines 198 to 204 in 5c1c48d
export function checkSortIsTimeBasedDescending(sortBys: Sort[]) { | |
return ( | |
getTimeBasedSortBy(sortBys) !== undefined && | |
sortBys.some(sortBy => sortBy.kind === 'desc') | |
); | |
} |
Was this report helpful? Give feedback by reacting with 👍 or 👎
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah really nice work here
); | ||
}, [data]); | ||
|
||
// We setup a RAF loop to update the virtual timestamp smoothly to emulate real-time streaming. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should have a throttled RAF if we detect bad hardware or older browsers somehow. Maybe do 30fps?
Would be nice to have some stats (send logs?) to track metrics on this.
|
||
const refreshCallback = useRef(() => {}); // Since the interval fetches data, it's an infinite dependency loop. | ||
const sortBysString = JSON.stringify(sortBys); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably useMemo
this.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
### Summary This adds an auto-refresh toggle behind the 'ourlogs-live-refresh' flag. Auto-refresh can be enabled with filters so long as the sort is by time and descending. It works on 5 second polling, with a virtual time using RAF to emulate streaming. Filters are applied so data is queried in a ~30s second window behind our ingest delay. Auto-refresh will automatically disable itself under the following conditions: - You've hit a rate limit consistently over 3 polls (>1k logs/s) - You've hit an api error for any reason - You've hit the absolute timeout cap (10 minutes), you can re-enable it if you're actively using it - You change sort / filter etc. A subsequent PR will contain changes for the graph updates. #### Other - Added some memos and callbacks for performance reasons since the virtual timestamp updates fairly quickly - Modified delayed data to follow already set delay in graphs (will be needed in the next PR)
Summary
This adds an auto-refresh toggle behind the 'ourlogs-live-refresh' flag.
Auto-refresh can be enabled with filters so long as the sort is by time and descending. It works on 5 second polling, with a virtual time using RAF to emulate streaming. Filters are applied so data is queried in a ~30s second window behind our ingest delay.
Auto-refresh will automatically disable itself under the following conditions:
A subsequent PR will contain changes for the graph updates.
Other